Skip to content

Install script#10

Open
gs0510 wants to merge 2 commits intomainfrom
install_script
Open

Install script#10
gs0510 wants to merge 2 commits intomainfrom
install_script

Conversation

@gs0510
Copy link
Owner

@gs0510 gs0510 commented Feb 26, 2021

No description provided.

@gs0510 gs0510 requested a review from pitag-ha February 26, 2021 12:04
@gs0510 gs0510 force-pushed the install_script branch 3 times, most recently from ba2ff3d to 58d01f3 Compare February 26, 2021 16:15
Copy link
Collaborator

@pitag-ha pitag-ha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks! I think getting the installation done is pretty tough. That's why I'm asking/nitpicking about everything I see.

I have three general questions:

  • We once talked about the possibility to run ofronds inside a container to avoid having to deal with different OS's. I think that might still be a good option. What do you think about that?
  • What would you think about doing some logging to inform the user about what's being done?
  • When would you clone the project? Would you do that in a separate script? I kind of assumed we would also do that in the installation script, but maybe there's a different place to do that.

install.sh Outdated
if [ -f "/etc/arch-release" ]; then #detect arch-linux
pacman -S opam
elif [[ "$OSTYPE" == "linux-gnu"* ]]; then
apt install opam
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can that be done without root privileges? (and similar for the other OS's)

install.sh Outdated
Comment on lines 3 to 35
# Install ocaml/opam
if [ -f "/etc/arch-release" ]; then #detect arch-linux
pacman -S opam
elif [[ "$OSTYPE" == "linux-gnu"* ]]; then
apt install opam
elif [[ "$OSTYPE" == "darwin"* ]]; then #MacOS
#Check if homebrew is installed
which=$(command -v which -s brew)
if [[ $which != 0 ]] ; then
# Install Homebrew
ruby -e "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)"
else
brew update
fi
brew install gpatch
brew install opam
elif [[ "$OSTYPE" == "freebsd"* ]]; then
cd /usr/ports/devel/ocaml-opam || exit
make install
elif [[ "$OSTYPE" == "openbsd"* ]]; then
pkg_add opam
else
"Unknown OS type, please install opam/ocaml manually."
fi
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can first check if opam is already installed before going ahead and installing it?

install.sh Outdated
fi

# Initailize a switch
opam init
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar: do you know if there's a way to check if that's already done?

install.sh Outdated
# Initailize a switch
opam init
eval "$(opam env)"
opam switch create 4.11.1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you prefer a global switch to a local one?

Also, do you think it might be possible to find the system compiler instead of always installing 4.11.1?

install.sh Outdated
eval "$(opam env)"
opam switch create 4.11.1
eval "$(opam env)"
opam install dune
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't that included in opam install .?

install.sh Outdated

# Initailize a switch
opam init
eval "$(opam env)"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for the case that the user didn't want to update their bash profile in opam init, right? If that happens, how do we solve the problem that dune might not be found when running the ofronds command? Should we maybe also run eval "$(opam env)" every time before internally running a dune command?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, that will be kind of annoying though, (to run eval everytime before running dune command). Do you know if we can enforce updating bash profile in opam init somehow?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could do that via opam init --auto-setup. We should discuss if that would be too intrusive. I think it should be fine.

@gs0510
Copy link
Owner Author

gs0510 commented Feb 27, 2021

We once talked about the possibility to run ofronds inside a container to avoid having to deal with different OS's. I think that might still be a good option. What do you think about that?

I think that's easier than getting the installation script to work for all OSes, though we'd also need to install docker, and it's not easy for all OSes (FreeBSD docker is tricky for example). I think introducing docker would increase the complexity and more chances of things going wrong (for now)

What would you think about doing some logging to inform the user about what's being done?

This is a good idea, will add more logging.

When would you clone the project? Would you do that in a separate script? I kind of assumed we would also do that in the installation script, but maybe there's a different place to do that.

In the README we can specify that folks should clone the project themselves (since the install script is inside the project it kind of creates a recursive problem).

@pitag-ha
Copy link
Collaborator

pitag-ha commented Mar 5, 2021

we'd also need to install docker, and it's not easy for all OSes (FreeBSD docker is tricky for example).

Oh ok, I didn't know about that. I thought installing docker was easier on different OS's than opam. The advantage of docker would still be that it could work relatively easily on Windows. But let's do without docker, at least for now, then.

In the README we can specify that folks should clone the project themselves (since the install script is inside the project it kind of creates a recursive problem).

Another option would be to do it similarly to rustlings: include the cloning of the project in the install script, host the install script separately and make a curl request to it for installation. That way, on unix systems, the whole set up would be just one line, something like (but adding the right flags):

curl <install_script_url> | bash

@gs0510
Copy link
Owner Author

gs0510 commented Mar 8, 2021

Ooh that's a good idea, we can also add installing git to the script if we have the install script hosted somewhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants