gap.sh should respect the GAP_DIR environment var #2465
Conversation
In previous versions of GAP, if the GAP_DIR environment variable was set, then GAP used this variable to determine the location of GAP. (Otherwise, it uses sensible defaults.) This patch returns to this behavior.
|
Out of interest, have you tried just running the GAP executable directly? The |
|
@ChrisJefferson, I in fact had tried running the gap executable directly, and hadn't even noticed that it found the library folder! (I was rather focused on testing fixes for the hardcoded library paths, which I've reported elsewhere.) I'll probably move to calling this directly in future versions of Gap.app. But if the purpose of gap.sh is compatibility with earlier versions of GAP, I think it should support GAP_DIR. Otherwise, it's completely dependent on hard-coded paths. |
Codecov Report
@@ Coverage Diff @@
## master #2465 +/- ##
==========================================
+ Coverage 73.98% 74.19% +0.21%
==========================================
Files 484 484
Lines 245407 245530 +123
==========================================
+ Hits 181566 182181 +615
+ Misses 63841 63349 -492
|
cnf/compat/gap.sh.in
Outdated
| GAPDIR=$(cd "@abs_top_srcdir@" && pwd) | ||
| exec "@abs_top_builddir@/gap" -l "$GAPDIR" "$@" | ||
| GAP_EXE=$GAP_DIR | ||
| if [ "x$GAP_DIR" = "x" ]; then |
There was a problem hiding this comment.
Please remove the extra space after ;
cnf/compat/gap.sh.in
Outdated
| GAP_EXE=$GAP_DIR | ||
| if [ "x$GAP_DIR" = "x" ]; then | ||
| GAP_DIR=$(cd "@abs_top_srcdir@" && pwd) | ||
| GAP_EXE=@abs_top_builddir@ |
There was a problem hiding this comment.
please indent the two preceding lines (either with two or four spaces each)
|
Apologies for taking a day or two to get back. I made the requested formatting updates. Backporting would indeed be sensible, as this patch will be mainly interesting in the short term. (I'm supposing that in the long term, everyone will just launch $GAP_DIR/gap directory.) Should I have done something different when submitting the pull request to request backporting? |
|
@RussWoodroofe : We have a tag for backporting, but discussing it in the PR is fine! |
|
Backported to stable-4.9 via 20dd8f1 |
olexandr-konovalov
added
the
release notes: added
In previous versions of GAP, if the GAP_DIR environment variable was set, then GAP used this variable to determine the location of GAP. (Otherwise, it uses sensible defaults.) This patch returns to this behavior.