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

Make 0install-solver available in the opam 2.1 installation #102

Merged
merged 3 commits into from
May 11, 2021

Conversation

kit-ty-kate
Copy link
Contributor

No description provided.

@talex5
Copy link
Contributor

talex5 commented Apr 21, 2021

Looks like this depends on a commit that isn't merged yet.

@kit-ty-kate
Copy link
Contributor Author

yes, it's on purpose, I remember @avsm saying starting with docker-base-images is better to avoid issues, at the last meeting

@avsm
Copy link
Member

avsm commented Apr 21, 2021

Yep, that's right. It's best to ensure things build here and then merge them (immediately) since we dont have a CI-for-the-CI. I'm just doing the multicore changes and then will look at this...

@avsm
Copy link
Member

avsm commented Apr 21, 2021

@kit-ty-kate wouldn't this be better as a patch to opam itself? I'm a little unclear on what's going on here -- we install the external 0install solver, and then it becomes available to opam as builtin-0install?

@kit-ty-kate
Copy link
Contributor Author

The required PR in opam has now been merged so this PR is now ready to be reviewed.

@talex5
Copy link
Contributor

talex5 commented Apr 28, 2021

The ocaml-dockerfile PR needs rebasing on master before we can push this to live, or we'll lose the other changes.

@kit-ty-kate
Copy link
Contributor Author

The ocaml-dockerfile PR needs rebasing on master before we can push this to live, or we'll lose the other changes.

Done but I'm not sure why Github is still saying that there is a conflict :/ the ocaml-dockerfile submodule on master is ocurrent/ocaml-dockerfile@617dbf7 and I've rebased my PR on top of ocaml-dockerfile master (which is just above ocurrent/ocaml-dockerfile@617dbf7). Any clue?

@talex5
Copy link
Contributor

talex5 commented Apr 28, 2021

This PR doesn't seem to have been rebased on the latest master. Its parent should be 753d0e2 (master) or ced6ed3 (live, testing the 16.04 PR), not 27f3330 (which is ancient).

@kit-ty-kate
Copy link
Contributor Author

Oh I see, I was only looking at ocaml-dockerfile, I didn't know changes in docker-base-images would affect this. git pull --rebase origin master fixed the issue without any conflict.

Hopefully it should be ready now, sorry for the setbacks.

@talex5
Copy link
Contributor

talex5 commented Apr 28, 2021

Deployed to live and building now...

@kit-ty-kate
Copy link
Contributor Author

Step 14/63 : RUN git clone -b master git://github.com/ocaml/opam /tmp/opam
 ---> Using cache
 ---> eb2d5bccece0
 [...]
env PATH="`pwd`/bootstrap/ocaml/bin:$PATH" ./configure --enable-cold-check --with-0install-solver
configure: WARNING: unrecognized options: --with-0install-solver

mmh, I think this just hit something akin to #104 :/

@talex5
Copy link
Contributor

talex5 commented Apr 28, 2021

Hmm, yes, this line needs to include the hash of the commit it wants.

@kit-ty-kate
Copy link
Contributor Author

This PR is ready for another round of review. Incidently it should also partly fix #104

Copy link
Contributor

@talex5 talex5 left a comment

Choose a reason for hiding this comment

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

This is a good change, but I think there is a small problem: it will rebuild all the images every time one of the opam branches or opam-repository updates.

Having everything on the weekly schedule could help if they all line up, but they won't in general (and if someone manually rebuilds one of them they'll get out-of-sync again).

We should probably have the three commit inputs stay up-to-date, but have them all go into a snapshot box that takes a snapshot of them once a week.

@kit-ty-kate
Copy link
Contributor Author

I spent the past week and a half trying various patches to ocurrent to make the ~schedule argument sync properly but the best I've been able to do is to sync 2 out of 3 inputs in my test case.

So in the meantime this gets resolved, I used a single cache entry to assemble the three of them in this particular context.
This PR is ready to review again.

Copy link
Contributor

@talex5 talex5 left a comment

Choose a reason for hiding this comment

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

This seems reasonable to get things unblocked. I've pushed it to live.

@talex5
Copy link
Contributor

talex5 commented May 10, 2021

Doesn't work, though:

2021-05-10 14:49.46: Exec: "git" "clone" "git://github.com/ocaml/opam" "."
Cloning into '.'...
2021-05-10 14:49.51: Exec: "git" "rev-parse" "2.0"
fatal: ambiguous argument '2.0': unknown revision or path not in the working tree.

https://base-images.ocamllabs.io/job/2021-05-10/144920-git-repositories-01ff68

@kit-ty-kate
Copy link
Contributor Author

oops, sorry for that. It's fixed.

@talex5
Copy link
Contributor

talex5 commented May 10, 2021

Still doesn't work:

Cloning into '/tmp/opam'...
fatal: not a git repository (or any of the parent directories): .git

Does it work for you locally when you test it with docker-compose up?

@kit-ty-kate
Copy link
Contributor Author

Does it work for you locally when you test it with docker-compose up?

Sorry for that, I really have no disk space to give to docker on my local machine and until now I didn't know my dev work server had its ports open. I had never had the need to test docker-base-images locally in the past so i didn't know how to test it either. Now i know, sorry for that.

I've now tested that at least the default alpine image is built all the way and contains opam 2.1 with the 0install solver enabled.

@talex5
Copy link
Contributor

talex5 commented May 10, 2021

OK, seems to be working now! Just needs ocurrent/ocaml-dockerfile#43 merging first.

It seems to take a long time to get to 0install though!

$ opam-2.1 install --dry-run --debug --solver=builtin-0install utop
...
00:07.351  0install               Solution found. Solve took 0.31 s

@avsm
Copy link
Member

avsm commented May 10, 2021

Merged the dockerfile pr. Isn't a chunk of that time opam-2.1 doing a format upgrade on ~/.opam from 2.0, @talex5?

@talex5
Copy link
Contributor

talex5 commented May 11, 2021

Isn't a chunk of that time opam-2.1 doing a format upgrade on ~/.opam from 2.0

I thought I ran that command multiple times and it was always slow.

@talex5 talex5 merged commit 6266998 into ocurrent:master May 11, 2021
@kit-ty-kate
Copy link
Contributor Author

It seems to take a long time to get to 0install though!

yeah it's not the upgrade to 2.1 (with the upgrade it take 17s to get to it), most of the bulk of the time is spent in the transformation to CUDF (4s), untaring the repository (2s) and detecting the depexts (1s):

00:00.960  XSYS                   Adding to env { LC_ALL=C }
00:02.088  SYSTEM                 mkdir /tmp/opam-226-3c100d
00:02.107  SYSTEM                 rmdir /tmp/opam-226-3c100d
00:02.118  STATE                  depexts loaded in 1.158s
00:02.506  STATE                  Detected changed packages (marked for reinstall): {}
00:02.511  CLIENT                 Base orphans: {}
00:02.527  CLIENT                 Orphans: (changes: { utop.1.2.1, utop.1.3.0, utop.1.4.0, utop.1.5, utop.1.6, utop.1.7, utop.1.8, utop.1.9, utop.1.10, utop.1.11, utop.1.12, utop.1.14, utop.1.15, utop.1.16, utop.1.17, utop.1.18, utop.1.18.1, utop.1.18.2, utop.1.19, utop.1.19.1, utop.1.19.2, utop.1.19.3, utop.2.0.0, utop.2.0.1, utop.2.0.2, utop.2.1.0, utop.2.2.0, utop.2.3.0, utop.2.4.0, utop.2.4.1, utop.2.4.2, utop.2.4.3, utop.2.5.0, utop.2.6.0, utop.2.7.0 }, transitive: false) -> full {}, versions {}
00:03.093  SOLVER                 resolve request=install:(utop) remove:() upgrade:()
00:04.545  SOLVER                 Load cudf universe (depopts:false, build:true, post:true)
00:04.834  CUDF                   resolve request=install:(utop) remove:() upgrade:()
00:08.344  CUDF                   Conflicts: 591 (533) pkgs to remove
00:08.367  CUDF                   Preprocess cudf request (trimming: simple): from 18036 to 17443 packages in 0.50s
00:08.367  SOLVER                 Calling solver builtin-0install with criteria 
00:08.700  0install               Solution found. Solve took 0.33 s
00:08.700  CUDF                   Solver call done in 0.830s

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.

3 participants