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

Update ocaml on Windows: add opam 2.2 #2727

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Conversation

RadioPotin
Copy link
Contributor

Greetings,

@rjbou and myself have updated OCaml on Windows tutorial to add opam workflows as the recommended one for installing OCaml on Windows.

Best regards,

@cuihtlauac
Copy link
Collaborator

cuihtlauac commented Sep 27, 2024

Beautiful! Thank you very much, @RadioPotin and @rjbou, this is awesome.
@Sudha247, @dra27, @jonahbeckford: any views on this?

Copy link
Contributor

@Sudha247 Sudha247 left a comment

Choose a reason for hiding this comment

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

Wonderful, this reads very nicely! Huge +1 from my side. Some minor comments below.

One detail to address about having a fully functional opam installation is
related to Git.

On Windows, there are many ways to have a functioning Git installation. Opam will
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't it recommended to obtain git via winget, too?

Copy link
Contributor

Choose a reason for hiding this comment

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

It is indeed. This sentence purpose is to introduce the Git opam init section.

@@ -92,6 +286,8 @@ easier way to get a working Windows environment on your machine.

### Visual Studio Code on Windows

**If you use opam installation**, you will need to add opam switch prefix on your path that runs VSCode.
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps there could be a link here on how to add switch prefix to the path?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is that needed at all? If it is needed, we should file a bug somewhere since that is quite unpleasant for a user.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll ping @dra27 on this one

installer that we recommend for new users. However, while [DkML] has a modern OCaml 4.14.0 compiler,
## opam

Opam now features a fully native Windows compatible installation process that we
Copy link
Contributor

Choose a reason for hiding this comment

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

I get the sentiment, but it isn't accurate to describe a Cygwin installation process as a fully native Windows compatible installation process. IMHO the real win is how easy it is to install Windows with opam 2.2 ... very few questions asked, no prereqs other than Git, etc. ... and the ease of install could be a good lead sentence.

fully functional OCaml compiler available to you:

```shell-session
> for /f "tokens=*" %i in ('opam env --switch=default') do @%i
Copy link
Contributor

Choose a reason for hiding this comment

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

Correct me if I'm wrong, but if you install winget in a terminal, and then install opam using winget, any subsequent opam like opam env will fail with Not Found, right? Because opam won't be in the PATH. If so, I think we need to insert a step to close and reopen the terminal.

Copy link
Contributor

Choose a reason for hiding this comment

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

True, this is specified at the beginning:

Once the package is installed, you can launch a new shell to have access to
your fresh opam binary.

Here, it is have access to OCaml after opam init, so we already have a terminal that have an opam binary in the path.

supporting an external dependency installation for Windows and integrating it with the
Windows shell. From an `opam-repository` perspective, the `ocaml-base-compiler`
packages will support the MinGW-w64 and MSVC variants.

## Installation Environments

### `opam-repository-mingw`
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 can remove this section entirely since it is deprecated and opam-repository is now compatible with Windows.

Copy link
Contributor

@rjbou rjbou left a comment

Choose a reason for hiding this comment

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

Thanks all for the reviews! I've addressed some in comments.

data/tutorials/getting-started/3_01_ocaml_on_windows.md Outdated Show resolved Hide resolved
data/tutorials/getting-started/3_01_ocaml_on_windows.md Outdated Show resolved Hide resolved
One detail to address about having a fully functional opam installation is
related to Git.

On Windows, there are many ways to have a functioning Git installation. Opam will
Copy link
Contributor

Choose a reason for hiding this comment

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

It is indeed. This sentence purpose is to introduce the Git opam init section.

fully functional OCaml compiler available to you:

```shell-session
> for /f "tokens=*" %i in ('opam env --switch=default') do @%i
Copy link
Contributor

Choose a reason for hiding this comment

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

True, this is specified at the beginning:

Once the package is installed, you can launch a new shell to have access to
your fresh opam binary.

Here, it is have access to OCaml after opam init, so we already have a terminal that have an opam binary in the path.

@RadioPotin
Copy link
Contributor Author

Hey again everybody,

Thank you ever so much for the constructive suggestions, hopefully this version is now mergeable,

Kind regards,

Copy link

@qrpnxz qrpnxz left a comment

Choose a reason for hiding this comment

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

A couple of details still make reference to a future release of opam 2.2.
Installation of git at the same time as opam should be suggested.

Comment on lines 212 to 213
it does not track the latest OCaml compilers. We will officially support Windows as a Tier 1
platform with a [major release of opam](#opam-22) in the coming months, and it will be compatible with
Copy link

Choose a reason for hiding this comment

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

We will officially support Windows as a Tier 1 platform with a [major release of opam](#opam-22) in the coming months,

Already happened, hence this PR. This link will be broken.

@@ -38,21 +239,12 @@ The guidance is based on the availability table below:
╰──────────────────────────────────────────────────────────────────────────────────────────╯
Copy link

Choose a reason for hiding this comment

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

│ Tier 1 │ OCaml 5 with Opam 2.2 │ Full support. Coming in the next few months

Already happened.

Comment on lines 127 to 128
Cygwin Git is functional but can have credentials issues for private repositories, we recommend using:
- Install via 'winget install Git.Git'
Copy link

Choose a reason for hiding this comment

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

This would be good to know from the outset. https://ocaml.org/install#windows recommends doing

winget install Git.Git OCaml.opam

RadioPotin and others added 8 commits October 10, 2024 15:22
Co-authored-by: Raja Boujbel <raja.boujbel@ocamlpro.com>
Co-authored-by: R. Boujbel <rjbou@ocamlpro.com>
Co-authored-by: R. Boujbel <rjbou@ocamlpro.com>
Co-authored-by: R. Boujbel <rjbou@ocamlpro.com>
Co-authored-by: Jonah Beckford <9566106-jonahbeckford@users.noreply.gitlab.com>
- Change wording of the introduction
- Recommend using winget install Git.Git OCaml.opam
- Shorten instructions when they can be made more concise
@tmattio
Copy link
Collaborator

tmattio commented Oct 10, 2024

I've pushed a few changes:

  • Integrated changes from Diskuv OCaml docs when OCaml 5 is released #554
  • Reworded the introduction as suggested by @jonahbeckford
  • Updated the availability table (thanks @qrpnxz)
  • Updated the recommended instructions to be winget install Git.Git OCaml.opam (thanks @qrpnxz)
  • Removed the stdout from the shell blocks, this was making the document very verbose for not much value I think
  • Shortened some instructions when they could be made more concise.

I think this is ready to go, but I'll wait for a thumbs up from @RadioPotin @rjbou if the changes above look good to you, and give a couple days for a last round of review.

Copy link
Contributor

@rjbou rjbou left a comment

Choose a reason for hiding this comment

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

Thanks all for these inputs!
I'm not sure for the removal of shell output: as pointed out Jonah, it helps users to know should happen, but if the rest of the website is without shell output, it's better to remove indeed.
Remain tiny comments and we should be good to go.

data/tutorials/getting-started/3_01_ocaml_on_windows.md Outdated Show resolved Hide resolved
data/tutorials/getting-started/3_01_ocaml_on_windows.md Outdated Show resolved Hide resolved
data/tutorials/getting-started/3_01_ocaml_on_windows.md Outdated Show resolved Hide resolved
@@ -92,6 +286,8 @@ easier way to get a working Windows environment on your machine.

### Visual Studio Code on Windows

**If you use opam installation**, you will need to add opam switch prefix on your path that runs VSCode.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'll ping @dra27 on this one

tmattio and others added 2 commits October 11, 2024 11:21
Co-authored-by: R. Boujbel <rjbou@ocamlpro.com>
Co-authored-by: R. Boujbel <rjbou@ocamlpro.com>
@tmattio
Copy link
Collaborator

tmattio commented Oct 11, 2024

Thanks @rjbou! All suggestions applied. We can bring back the shell output if others think this contributes to the clarity of the document, but indeed, we don't do that in the rest of the documentation.

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.

8 participants