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

Add graphics package to existing switches #12674

Merged
merged 3 commits into from
Mar 14, 2019
Merged

Conversation

dra27
Copy link
Member

@dra27 dra27 commented Sep 22, 2018

At present, the graphics package allows the absence of the OCaml graphics library to be detected as a dependency failure (which is neater than a build system error). This PR enhances that by building and installing the graphics library if it's missing. Here's an example:

dra@bionic:~$ ocamlfind query graphics
ocamlfind: Package `graphics' not found

dra@bionic:~$ opam install graphics
The following actions will be performed:
  ∗ install graphics 1.0

<><> Gathering sources ><><><><><><><><><><><><><><><><><><><><><><><><><><><><>

<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
[ERROR] The compilation of graphics failed at "/home/dra/.opam/opam-init/hooks/sandbox.sh build ocamlc -custom graphics.cma -o test".

#=== ERROR while compiling graphics.1.0 =======================================#
...
<><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><>
┌─ The following actions failed
│ λ build graphics 1.0
└─ 
╶─ No changes have been performed

<><> graphics.1.0 troubleshooting <><><><><><><><><><><><><><><><><><><><><><><>
=> This package checks whether the Graphics library was compiled.

dra@bionic:~$ opam remote add graphics git+https://github.com/dra27/opam-repository.git#graphics
...

dra@bionic:~$ opam install graphics --yes
The following actions will be performed:
  ∗ install conf-pkg-config 1.1    [required by conf-libX11]
  ∗ install conf-libX11     1      [required by graphics]
  ∗ install graphics        4.06.1
===== ∗ 3 =====

<><> Gathering sources ><><><><><><><><><><><><><><><><><><><><><><><><><><><><>
[graphics.4.06.1] found in cache

<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
∗ installed conf-pkg-config.1.1
∗ installed conf-libX11.1
∗ installed graphics.4.06.1
Done.

dra@bionic:~$ ocamlfind query graphics
/home/dra/.opam/ocaml-with-graphics/lib/graphics

For system switches, the library is installed as a findlib package only so, just as with nums in 4.06.0 onwards, users of system switches will need to be using something more sophisticated than #load "graphics.cma". For opam-built compilers, they are installed to the OCaml libdir as OCaml would - the package installs a META file if ocamlfind has already been installed (ocamlfind will automatically generate META on any future installations).

A conf-libX11 package is added which at the moment is based entirely on pkg-config. Part of removing graph and win32graph from OCaml may include transferring the configure logic to this package, but for now I kept it simple.

Reviewers may wish to note that the install.sh is identical in all packages and the opam file differs only by version-specific patches. Originally I did this all in opam files, but it's really difficult to deal with both system and non-system compilers and the fact that graphics may be already installed in the commands, so a script seemed somewhat less unclear.

These packages were generated by the scripts in dra27@14f9326.

@camelus
Copy link
Contributor

camelus commented Sep 22, 2018

☀️ All lint checks passed 0c4c0de
  • These packages passed lint tests: conf-libX11.1, graphics.3.07+1, graphics.3.07+2, graphics.3.07, graphics.3.08.0, graphics.3.08.1, graphics.3.08.2, graphics.3.08.3, graphics.3.08.4, graphics.3.09.0, graphics.3.09.1, graphics.3.09.2, graphics.3.09.3, graphics.3.10.0, graphics.3.10.1, graphics.3.10.2, graphics.3.11.0, graphics.3.11.1, graphics.3.11.2, graphics.3.12.0, graphics.3.12.1, graphics.4.00.0, graphics.4.00.1, graphics.4.01.0, graphics.4.02.0, graphics.4.02.1, graphics.4.02.2, graphics.4.02.3, graphics.4.03.0, graphics.4.04.0, graphics.4.04.1, graphics.4.04.2, graphics.4.05.0, graphics.4.06.0, graphics.4.06.1, graphics.4.07.0, graphics.4.07.1

☀️ Installability check (10785 → 10822)
  • new installable packages (37): conf-libX11.1 graphics.3.07 graphics.3.07+1 graphics.3.07+2 graphics.3.08.0 graphics.3.08.1 graphics.3.08.2 graphics.3.08.3 graphics.3.08.4 graphics.3.09.0 graphics.3.09.1 graphics.3.09.2 graphics.3.09.3 graphics.3.10.0 graphics.3.10.1 graphics.3.10.2 graphics.3.11.0 graphics.3.11.1 graphics.3.11.2 graphics.3.12.0 graphics.3.12.1 graphics.4.00.0 graphics.4.00.1 graphics.4.01.0 graphics.4.02.0 graphics.4.02.1 graphics.4.02.2 graphics.4.02.3 graphics.4.03.0 graphics.4.04.0 graphics.4.04.1 graphics.4.04.2 graphics.4.05.0 graphics.4.06.0 graphics.4.06.1 graphics.4.07.0 graphics.4.07.1

@dra27
Copy link
Member Author

dra27 commented Sep 23, 2018

The Camelus broken package is because this PR assumes #12669.

#12451 fixed the checksum for the clang patch in 4.00.1, but for some reason skipped 4.01.0, so that's bundled in here.

I've added stuff which seems to fix pkg-config detection of X11 on macOS!

I'm at a loss as to how to make DataKitCI happy - the first time it installed conf-pkg-config and conf-libX11 and complained at ./install.sh in the graphics package. So I changed it to be sh ./install.sh but then DataKitCI the second time suddenly can't install conf-pkg-config. Meh.

@kit-ty-kate
Copy link
Member

#=== ERROR while compiling graphics.4.07.0 ====================================#
"./install.sh": command not found.

I guess there is a chmod +x missing in this file

@kit-ty-kate
Copy link
Member

I don't understand the problem.. :/ The file already has the right permissions.. Anyone has any idea?

@dra27
Copy link
Member Author

dra27 commented Oct 5, 2018

I tried a test commit changing the call to be sh ./install.sh but then I got errors about being unable to install conf-pkg-config instead :-/

@dra27
Copy link
Member Author

dra27 commented Oct 5, 2018

Basically, there are various things which could be going on (error in opam’s logic to find the command, error in the script, etc.), but I gave up when it appeared that the process wasn’t stable.

@dra27
Copy link
Member Author

dra27 commented Oct 5, 2018

@samoht - any ideas? avsm I know is way too snowed under to be able to look at this atm (of course, you may be too!)

@dra27
Copy link
Member Author

dra27 commented Nov 1, 2018

I think I may have solved this - please don't merge, even if CI passes this time round

@kit-ty-kate
Copy link
Member

Ahhh

[WARNING] Could not read file /home/opam/.opam/repo/default/packages/graphics/graphics.4.07.1/opam. skipping:
          At /home/opam/.opam/repo/default/packages/graphics/graphics.4.07.1/opam:30:2:
          Parse error

Somehow it's a silent error :/

@kit-ty-kate
Copy link
Member

I've pushed a fix to get the error sooner next time: avsm/mirage-ci@5d761ef

@dra27
Copy link
Member Author

dra27 commented Nov 1, 2018

Thanks, @kit-ty-kate! The log now reveals the problem as well:

Processing  5/6: [graphics: ls install.sh]
+ /bin/ls "-l" "install.sh" (CWD=/home/opam/.opam/4.07/.opam-switch/build/graphics.4.07.1)
- -rw-r--r-- 1 opam opam 1980 Nov  1 18:57 install.sh
Processing  5/6: [graphics: ocaml unix.cma]
+ /home/opam/.opam/4.07/bin/ocaml "unix.cma" "probe.ml" (CWD=/home/opam/.opam/4.07/.opam-switch/build/graphics.4.07.1)
- uid=1000  groups=  stat=1000,1000,644

#=== ERROR while compiling graphics.4.07.1 ====================================#
"./install.sh": command not found.

The error message from opam should be better (ocaml/opam#3649), but why is the file system not correct for DataKit CI?

@dra27
Copy link
Member Author

dra27 commented Nov 1, 2018

OK, not sure how much more poking around I can sensibly do: https://ci.ocamllabs.io/log/saved/docker-build-2b6c0b6fad0045aab224e73d3691e9d1/e4aa5092ef8815c8d2ddad5b249ea940689f19ce shows that the install.sh files are definitely checked out from git with mode 755 but by the time they're ending up in the build directory they've lost them. Is one of the intermediate container snapshots not preserving execute bits for some reason?

@dra27 dra27 force-pushed the graphics branch 2 times, most recently from 4ac1b79 to 298086e Compare March 13, 2019 11:59
@dra27 dra27 mentioned this pull request Mar 13, 2019
dra27 added 2 commits March 13, 2019 14:28
If the OCaml graphics library is not installed, it is installed to
ocamlc -where (for opam-built compilers) or as a findlib package (for
system compilers).
@kit-ty-kate
Copy link
Member

This looks ok. Should I go ahead and merge it?

@dra27
Copy link
Member Author

dra27 commented Mar 14, 2019

Huzzah - finally green! Please do 🙂

@kit-ty-kate
Copy link
Member

Thanks a lot!!

@kit-ty-kate kit-ty-kate merged commit 7049f73 into ocaml:master Mar 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants