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

Test OCaml for Windows #32

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft

Test OCaml for Windows #32

wants to merge 9 commits into from

Conversation

MisterDA
Copy link
Contributor

@MisterDA MisterDA commented Jan 14, 2022

Using Obuilder Docker on Windows backend and ocaml/opam Windows
images, currently with opam 2.0.8 and fdopen/opam-repository-mingw.

Server configuration:

default-repository: fdopen/opam-repository-mingw#opam2
list-command: ocaml-env exec --64 -- opam list --available --installable --short --all-versions
platform:
  os: windows
  arch: x86_64
  distribution: windows-mingw
  image: ocaml/opam:windows-mingw@sha256:6da6cc25ee75b314d3ef12dfe178f6060940e3c43202541d956f88c8c36b2ab3

@MisterDA MisterDA changed the title Test OCaml for Windows and opam-repo-ci Test OCaml for Windows Jan 14, 2022
Copy link
Contributor

@kit-ty-kate kit-ty-kate left a comment

Choose a reason for hiding this comment

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

My goal for the windows testing is to only support upstream so I won’t merge anything that uses fdopen’s fork. Same for duplicated functions, that would be a maintenance nightmare otherwise ^^"
Any chance you could add opam-dev to the docker image?

Comment on lines 168 to 182
let with_lower_bound_linux pkg = {|
if [ $res = 0 ]; then
opam remove -y "|}^pkg^{|"
env OPAMCRITERIA="+removed,+count[version-lag,solution]" opam install -vy "|}^pkg^{|"
res=$?
fi
|}

let with_lower_bound_windows pkg = {|
if [ $res = 0 ]; then
ocaml-env exec --64 -- opam remove -y "|}^pkg^{|"
ocaml-env exec --64 -- env OPAMCRITERIA="+removed,+count[version-lag,solution]" opam install -vy "|}^pkg^{|"
res=$?
fi
|}
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of duplicating every functions I think we should aim at factorising things. For example:

Suggested change
let with_lower_bound_linux pkg = {|
if [ $res = 0 ]; then
opam remove -y "|}^pkg^{|"
env OPAMCRITERIA="+removed,+count[version-lag,solution]" opam install -vy "|}^pkg^{|"
res=$?
fi
|}
let with_lower_bound_windows pkg = {|
if [ $res = 0 ]; then
ocaml-env exec --64 -- opam remove -y "|}^pkg^{|"
ocaml-env exec --64 -- env OPAMCRITERIA="+removed,+count[version-lag,solution]" opam install -vy "|}^pkg^{|"
res=$?
fi
|}
let with_lower_bound ~windows pkg =
let opam = if windows then "ocaml-env exec --64 -- opam" else "opam" in
{|
if [ $res = 0 ]; then
|}^opam^{| remove -y "|}^pkg^{|"
|}^opam^{| install --criteria="+removed,+count[version-lag,solution]" -vy "|}^pkg^{|"
res=$?
fi
|}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's sensible for these small functions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it not possible for the main function?

Using Obuilder Docker on Windows backend and ocaml/opam Windows
images, currently with opam-dev 2.0 and fdopen/opam-repository-mingw.

Server configuration:

```yaml
default-repository: fdopen/opam-repository-mingw#opam2
list-command: ocaml-env exec --64 -- opam list --available --installable --short --all-versions
platform:
  os: windows
  arch: x86_64
  distribution: windows-mingw
  image: ocaml/opam:windows-mingw@sha256:6da6cc25ee75b314d3ef12dfe178f6060940e3c43202541d956f88c8c36b2ab3
```
@MisterDA MisterDA force-pushed the windows branch 2 times, most recently from 85053c5 to fca0ced Compare February 15, 2023 10:50
@MisterDA MisterDA force-pushed the windows branch 3 times, most recently from bdc25ec to 4f4e9e2 Compare February 15, 2023 12:32
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