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

Remove uninstallable packages #11400

Closed
wants to merge 1 commit into from
Closed

Remove uninstallable packages #11400

wants to merge 1 commit into from

Conversation

AltGr
Copy link
Member

@AltGr AltGr commented Feb 12, 2018

cf. #11378

@camelus
Copy link
Contributor

camelus commented Feb 12, 2018

✅ No new or changed opam files 300fa51

✅ Installability check (8292 → 8265)
  • removed broken packages (38): async.108.00.01 async_core.108.00.01 async_extra.108.00.01 async_unix.108.00.01 github-jsoo.3.0.0 gpr.1.3.0
    jitsu.0.2 jitsu.0.3.0 jitsu-libvirt.0.0.1 jitsu-libxl.0.0.1 jitsu-xapi.0.0.1 mirage-http-xen.1.0.0 ocaml-top.1.0.0 ocaml-top.1.0.1
    ocaml-top.1.1.0 ox.1.0.0 ox.1.0.1 sociaml-tumblr-api.0.1.0 topkg-care.0.7.5 topkg-care.0.7.6 topkg-care.0.7.7 topkg-care.0.7.8
    topkg-care.0.7.9 topkg-care.0.8.0 topkg-care.0.8.1 vhd-format.0.7.0 vhd-format.0.7.1 vhd-tool.0.7.1 vhd-tool.0.7.2 vhd-tool.0.7.5
    vhd-tool.0.7.6 vhd-tool.0.7.7 webdav.1.1 xen-block-driver.0.2.3 xen-block-driver.0.2.4 xen-block-driver.0.2.5 xen-disk.1.0.2
    xen-disk.1.0.3
  • removed installable packages (27): bap.0.9.1 bap.0.9.2 bap.0.9.3 bap.0.9.4 bap.0.9.6 bap.0.9.7 gpr.1.2.2 mirage-fs-unix.1.0.0
    mirage-fs-unix.1.1.0 mirage-net-xen.0.9.0 mirage-net-xen.1.1.0 mirage-net-xen.1.1.1 mirage-net-xen.1.1.2 mirage-net-xen.1.1.3
    mirage-net-xen.1.2.0 mirage-net-xen.1.3.0 mirage-net-xen.1.4.0 mirage-net-xen.1.4.1 mirage-tcpip-xen.0.9.5 modelica_ml.0.2.0
    ospec.0.3.0 posixat.v0.9.1 release.1.0.2 stog.0.11.0 stog.0.12.0 stog-writing.0.12.0 utop.1.6

@AltGr
Copy link
Member Author

AltGr commented Feb 12, 2018

Some packages aren't detected as uninstallable by Camelus, because it's based on 1.2 and doesn't check the available field. For example:

  • bap and utop have ocaml-version = "4.01" and there is no such version
  • mirage-net-xen depends on ocaml<3.07, and 3.07 is the earliest packaged version

@AltGr
Copy link
Member Author

AltGr commented Feb 13, 2018

Pinging maintainers: @janestreet @ivg @vbmithr @dakk @samoht @Drup @mmottl @MagnusS @smondet @avsm @choeger @zoggy @andrenth @arjunguha @whitequark @dominicjprice @dbuenzli @diml @djs55 @jpdeplaix

I'll quickly merge this if there is no complaint (the packages can't be installed anyway! It'll always be time to re-add them if they get fixed).

@hannesm: do you still have a mapping of maintainer fields to github IDs ? Would be very convenient for PRs like this one :)

@dominicjprice
Copy link
Contributor

No complaint from me. For future reference, is there a procedure for the author to remove packages that are no longer maintained from opam?

@AltGr
Copy link
Member Author

AltGr commented Feb 13, 2018

There is no function opam unpublish at the moment, so no direct way: you would have to fork and clone the repository, git rm -r the packages, then push to your fork and file a Github pull-request.

@arjunguha
Copy link
Contributor

This is ok. Thanks.

@hannesm
Copy link
Member

hannesm commented Feb 13, 2018

@hannesm: do you still have a mapping of maintainer fields to github IDs ? Would be very convenient for PRs like this one :)

source is https://github.com/hannesm/conex/tree/master/analysis , data hannesm@0b15a5e .

@djs55
Copy link
Contributor

djs55 commented Feb 13, 2018

General comments:

  • it'd be good to highlight which of these are the latest existing package. For example I don't mind losing the older vhd-tool versions since I would recommend everyone use the latest version anyway.
  • it'd also be good to highlight when a package name would completely disappear from the namespace: once a name isn't taken any more it's more likely another package will take its place, preventing the original package from being easily fixed and reinstated, which could be confusing.
  • it'd be good to index the list by maintainer to make it easier for me to search :)

I notice one of the removed packages is functoria.2.2.0 which seems to be the latest version and required by the mirage tool (also removed). I had a quick look and both seem to install and work ok, but I think functoria has a typo in the test dependency: FYI @samoht @Drup

"alcotest" {test & >="2.0.0"}
-- is version 2.0.0 of alcotest expected? I'd prefer to fix these typos and keep these packages if possible.

Copy link
Member

@ivg ivg left a comment

Choose a reason for hiding this comment

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

What makes you think, that those packages are not installable? They were, in particular all bap packages were passing all opam CI tests, as well as ours. And I'm pretty sure, that I will not have any problems installing them today.

Also, I strongly disagree with the idea of deleting packages (the only exception should be the licensing issue).The OPAM repository should be a monotone data accumulator where the information is only accumulated and refined. By deleting packages we are sending a sad message to the community about the stability of the OCaml ecosystem and it's longlivity.

And this is not only about BAP, I'm also aware of many closed source systems, that use old versions of compilers and older software, and are still in use and will be in use for many years.

@vbmithr
Copy link
Contributor

vbmithr commented Feb 13, 2018

Yeah, what's the problem with i.e. secp256k1? A bit lost here…

@Drup
Copy link
Contributor

Drup commented Feb 13, 2018

Uhuh, not okay. abort! :<
Functoria and mirage works (the mirage community would have gathered with pitchforks otherwise). I'm pretty sure @djs55 got the right idea. I think your tool need some refinements @AltGr !

@AltGr
Copy link
Member Author

AltGr commented Feb 13, 2018

Ok, ok, please calm down :)
I had left the option to check for test/doc deps on, my bad. Packages still need fixing, though !
@djs55 identified the problem with functoria correctly:

% opam install functoria.2.2.0 --show --with-test --with-doc
The following dependencies couldn't be met:
  - functoria → alcotest >= 2.0.0
    no matching version

secp256k1 has a similar issue:

% opam install secp256k1.0.2.5 --show --with-test --with-doc
The following dependencies couldn't be met:
  - secp256k1 → ounit >= 2.2.2
    no matching version

I'll be updating the PR to fix that.

As for bap, only older releases are concerned. Here is what you get for 0.9.7:

% opam install bap.0.9.7 --show            
The following dependencies couldn't be met:
  - bap → ocaml = 4.01.0
  - bap → utop → ppx_tools → ocaml >= 4.02.0
  - bap → utop → ocaml (< 4.01.0 | >= 4.02.3)
  - bap → utop → cppo_ocamlbuild → jbuilder >= 1.0+beta10 → ocaml >= 4.02.3
Your request can't be satisfied:
  - No available version of ocaml satisfies the constraints

What makes you think, that those packages are not installable? They were, in particular all bap packages were passing all opam CI tests, as well as ours. And I'm pretty sure, that I will not have any problems installing them today.

See above, and the link in the OP for details. Apart from my mistake with enabling test/doc checks, the current repository has no possible configuration where you can install these packages. Probably there were at some point, maybe some dependencies got removed or changed — I am not trying to find any culprit here, and the solution might be to recover the dependencies — but the fact stands. Keeping packages that can't possibly be installed adds cruft and is, I think, misleading to the users.

Also, I strongly disagree with the idea of deleting packages

Well, I am not saying we should in general remove old package releases: opam was built around the possibility to have multiple compiler switches, including older releases, and that's certainly to remain built into its DNA. But we need to strike a balance between keeping working older releases, and not letting the repository look like an unmaintained dump — which wouldn't be a better signal to the community. I am only trying to tidy up here.

My other issue is the one explained in ocaml/opam#3161: I am pretty sure no one needs 48 releases for cohttp, and this number does have a cost. I am at the moment putting lots of effort into finding solutions and improving the solvers and the way we use them, but the installation problem remains NP-complete, so we are bound to hit issues at some point. This is why I am also putting efforts into cleaning up the repository, while retaining working build chains and maintained older releases of packages. I never really intended to merge #11194, but that's the idea :)

So you are right about keeping older releases and compatibility, and I completely agree there, the information should be "accumulated and refined" (so I won't be arguing that the repository is versionned). I am just focusing on the "refined" part ☺. Keeping old releases, yes, but keeping them unmaintained and in an unusable state, no. Let's keep the repository healthy, and while at it, if we can have it as lean as possible to avoid unneeded resolution problems, all the better.

follow-up from ocaml#11378. If nobody can install them, no point in keeping them in the repo
@AltGr
Copy link
Member Author

AltGr commented Feb 13, 2018

Here are the details for modelica.0.2.0, which is indeed the latest version:

% opam install --show modelica_ml.0.2.0 --sw empty
The following dependencies couldn't be met:
  - modelica_ml → ppx_deriving = 1.0 → ocaml = 4.02.1
  - modelica_ml → ppx_import → cppo_ocamlbuild → jbuilder >= 1.0+beta10 → ocaml >= 4.02.3
Your request can't be satisfied:
  - No available version of ocaml satisfies the constraints

@smondet
Copy link
Contributor

smondet commented Feb 13, 2018

@AltGr I can't find why I'm @mentioned in this one :)
(Also, I agree with @ivg on the fact that the opam-repo should be monotonically growing, maybe some tagging of the packages could tell the solver that it should not look at them by default)

@AltGr
Copy link
Member Author

AltGr commented Feb 14, 2018

@smondet: the PR originally removed packages that couldn't install with --with-test or --with-doc, but were fine otherwise, due to a mistake on my part. You were probably listed as maintainers of one of those, but not in the packages that really were uninstallable.

Here are the packages with doc/test issues from the original list. I omitted their dependencies, which transitively got marked as broken (among which ketrew, which doesn't actually have a problem — sorry):

bitcoinml.0.2 bitcoinml.0.2.4 Davide Gessa <gessadavide@gmail.com>
The following dependencies couldn't be met:
  - bitcoinml → ounit >= 2.2.2
    no matching version

functoria.2.2.0 Gabriel Radanne <drupyog@zoho.com>
The following dependencies couldn't be met:
  - functoria → alcotest >= 2.0.0
    no matching version

ppx_deriving_yojson.2.0 ppx_deriving_yojson.2.1 Peter Zotov <whitequark@whitequark.org>
The following dependencies couldn't be met:
  - ppx_deriving_yojson → ppx_deriving (>= 1.0 & < 2.0) → ocaml = 4.02.1
  - ppx_deriving_yojson → ppx_import → cppo_ocamlbuild → jbuilder >= 1.0+beta10 → ocaml >= 4.02.3
Your request can't be satisfied:
  - No available version of ocaml satisfies the constraints
  - No available version of ppx_deriving satisfies the constraints

secp256k1.0.2.5 secp256k1.0.3.0 secp256k1.0.3.1 secp256k1.0.3.2 Davide Gessa <gessadavide@gmail.com>
The following dependencies couldn't be met:
  - secp256k1 → ounit >= 2.2.2
    no matching version

As for the monotonic increase, we can have this discussion: is the repository an archive, or a living, maintained corpus of packages ? Because we have been allowing package modifications from the beginning, I had assumed we wanted the latter. To me, the repository's purpose is, more than anything else, to be used by opam to allow users to install packages. That does include older releases of packages, I have never been arguing that we should remove those. But to avoid code rot, we should tidy up from time to time, and make sure that the packages are installable, otherwise they can interfere with the opam tool, and are misleading to users. So I don't quite understand your position here: what could keeping broken packages, possibly achieve ?

As for the 48 releases of cohttp, it's a different matter, but I am pretty sure most of these could be removed without any impact on the users, as long as we keep all major releases. Really, when rolling a new installation, that much choice is not helpful, and there is bound to be some versions that are just bad choices (e.g. equivalent to others, but without important security fixes). Again, this is different from the matter of this PR, and I don't intend to do anything blindly. Just arguing that using a monotically increasing archive as our main repo may not be such a good idea in the long run.

@smondet
Copy link
Contributor

smondet commented Feb 14, 2018

@AltGr one of the goals of being monotone is to make sure opam update ; opam upgrade doesn't get users into a weird state.

Let's say conduit.0.5.1 is broken/uninstallable, and cohttp.0.4.2 depends on it.

Some user may have pinned conduit to some special branch or local-fix, and hence managed to install cohttp.0.4.2 (e.g. pinned to the version).

Now if those 2 packages get removed from the repo, opam update ; opam upgrade will get the user into a state where a package is installed but does not exist in the repo, right?

(I'm not saying this is a huge deal, I'm sure all those cases can be handled with good error messages, but in general, it's a nice assumption for the mental model of how opam works)

@AltGr
Copy link
Member Author

AltGr commented Feb 14, 2018

Now if those 2 packages get removed from the repo, opam update ; opam upgrade will get the user into a state where a package is installed but does not exist in the repo, right?

Indeed; but I don't see having the package installed but no longer present in the repository as an issue: opam keeps a copy of the metadata locally. If the package is pinned, nothing will happen (it's already independant from the repo), if it is not, opam will flag it as "orphan", and an upgrade command will prompt to move away from the orphaned version, or to remove the package if there are no other versions. Other commands that don't specifically interact with the package will leave it as is.

In general, having packages on the repository that depend on anything that is not in the repository (here a custom pinning) would break another assumption, that the repository should be self-contained.

@dakk
Copy link
Contributor

dakk commented Feb 15, 2018

I just fixed dep errors for secp256k1 and submitted a new release (I don't know why I set ounit-2.2.2 while the latest version is 2.0.7); old version can be removed

@UnixJunkie
Copy link
Contributor

isn't it bad for reproducibility that some packages can disappear from opam?

@AltGr
Copy link
Member Author

AltGr commented Feb 21, 2018

@dakk thanks

@UnixJunkie well, it's no worse than any packages modifications. You can lock at a given repository commit for reproducibility, and already need to do that anyway.

Also note that opam 1.2 didn't really work on this front, because one of the most common issues was changed or missing upstream sources. Opam 2.0 solves this by having a cache of source archives by hash, that is always appended to. This means that, with the cache properly configured, locking to an older repository version will actually work.

@ocaml ocaml deleted a comment from camelus Mar 27, 2018
@kit-ty-kate
Copy link
Member

I don't think there is going to be consensus on this any time soon. I'm going to close this. Feel free to reopen it or to create another one whenever you want.

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.

None yet