Closed mtitov closed 3 years ago
I was able to reproduce this. It is due to the fact that sh != bash on Ubuntu. This fix works for me:
index f842563..42c6519 100755
--- a/docker/swift-t/install-deps.sh
+++ b/docker/swift-t/install-deps.sh
@@ -4,7 +4,7 @@ set -eux
# INSTALL DEPENDENCIES
# Currently just Tcl
-if which tclsh8.6 >& /dev/null
+if which tclsh8.6 > /dev/null 2>&1
then
# Tcl is installed, probably via package manager.
# Note that this does not check for tcl-devel,
diff --git a/docker/swift-t/install-swift-t.sh b/docker/swift-t/install-swift-t.sh
index 2c6f2e8..6e02921 100755
--- a/docker/swift-t/install-swift-t.sh
+++ b/docker/swift-t/install-swift-t.sh
@@ -13,8 +13,7 @@ PATH=/opt/tcl-8.6.11/bin:$PATH
git clone https://github.com/swift-lang/swift-t.git
# Build it
-pushd swift-t/dev/build
+cd swift-t/dev/build
cp -v swift-t-settings.sh.template swift-t-settings.sh
sed -i "/SWIFT_T_PREFIX=/s@=.*@=$SWIFT_ROOT@" swift-t-settings.sh
./build-swift-t.sh
-popd
I think it makes sense to include this fix with this PR because this is part of Ubuntu compatibility.
Thank you, @j-woz ! That got resolved
@dongahn @andre-merzky @mturilli @j-woz adding python installation script brings another point for internal reorganization in base docker files, which should be in sync with #70 (and there is one more input parameter for future consideration - python version).
(1) for pip
- use python installation script and set virtualenv (..-m venv..
),
(2) for conda
- use conda installation script (maybe current code for conda in #70 could be also moved into a separate script? @zekemorton) and create conda env (conda create -p...
)
UPDATE (09/17)
@mtitov: is this ready to go in? As I remember the only things that were needed was 1) resolve Swif/T issues and 2) use the distro provided Python 3 RPM. Let me know.
@dongahn sorry for the delay, it is ready now p.s. even we switched back to default python, I still keep the python script in (just FYI)
Not a problem. If the python script isn't used, you can take it off and do a separate PR to be reviewed more quickly. But it's minor enough, I will trust your judgement. Thanks. Let's discuss tomorrow with a goal to land it soon.
Thanks @andre-merzky for reviewing and merging!
BTW, you could also set the merge-when-passing label and mergefyio handles it. (Could be the case a manual merge was required for this case though...)
BTW, you could also set the merge-when-passing label and mergefyio handles it. (Could be the case a manual merge was required for this case though...)
Ah, sorry, I forgot that this is policy here, will do next time! (manual merge was not necessary)
A manual merge is perfectly fine and this is not a problem at all. It is just that mergefyIO would do a bit more check for us. Robot :-)!!
Issues to resolve:
install-mpi.sh
used for CentOS7/8 is commented out)