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

Mitigate dune downgrades #14267

Closed

Conversation

kit-ty-kate
Copy link
Member

Mitigate #14185 by avoiding downgrades as much as possible. This should work for all opam packages in opam-repository but there might still be some issues for external packages or remotes.

This PR disables those packages from the repository and cannot be installed anymore:

  • decoders.0.1.0
  • decoders.0.1.1
  • lwt.3.1.0
  • lwt.3.2.0
  • lwt.3.2.1
  • lwt.3.3.0
  • ocp-index.1.1.6
  • ocp-index.1.1.7
  • yojson.1.4.0
  • yojson.1.4.1

None of these packages are their latest versions but this might also make some more packages uninstallable. This will be detected by Camelus.

cc @avsm @diml @AltGr

@camelus
Copy link
Contributor

camelus commented Jun 11, 2019

☀️ All lint checks passed a49a010
  • These packages passed lint tests: decoders.0.1.0, decoders.0.1.1, lwt.3.1.0, lwt.3.2.0, lwt.3.2.1, lwt.3.3.0, ocp-index.1.1.6, ocp-index.1.1.7, yojson.1.4.0, yojson.1.4.1

🌤️ Installability check (11202 → 11169)
  • these releases are not installable anymore (33): bap-server.0.2.0 core-lwt.0.2.0 decoders.0.1.0 decoders.0.1.1 fkie-cad-cwe-checker.0.1 flowtype.0.72.0
    flowtype.0.78.0 flowtype.0.79.0 flowtype.0.79.1 flowtype.0.80.0 flowtype.0.87.0 flowtype.0.93.0 flowtype.0.94.0 horned_worm.0.1.1 learn-ocaml.0.10 lwt.3.1.0
    lwt.3.2.0 lwt.3.2.1 lwt.3.3.0 nsq.0.2.4 nsq.0.2.5 nsq.0.3.0 ocp-browser.1.1.6 ocp-browser.1.1.7 ocp-browser.1.1.8 ocp-index.1.1.6 ocp-index.1.1.7
    otetris.1.0 otetris.1.1 plotkicadsch.0.2.0 plotkicadsch.0.3.0 yojson.1.4.0 yojson.1.4.1

@kit-ty-kate kit-ty-kate force-pushed the mitigate-dune-downgrades branch from 735c774 to a49a010 Compare June 11, 2019 14:05
@mseri
Copy link
Member

mseri commented Jun 13, 2019

I understand your reasons, but I am worried by this solution. For example lwt.3 is not that old, and it was the release before the big breakage: there are many packages out (some of which not here but using opam) that work with it but not with lwt 4.

@ghost
Copy link

ghost commented Jun 13, 2019

To give a bit more context, this PR follows a discussion we had this week with @kit-ty-kate, @AltGr and @avsm. I raised some concerns about #14266 as I was worried about the impact for opam and dune users.

We then had a discussion between dune developers yesterday to see if we could avoid this problem entirely by making dune forward compatible. It turns out that while backward compatibility is good and already supported by dune, forward compatibility introduces other issues for both dune and users of dune. Ideally, you shouldn't have to rebuild everything when you upgrade dune, but when you downgrade dune it's cleaner and safer to rebuild everything. We can't express that fact in opam files at the moment, so I suppose the safest approach is @kit-ty-kate's original PR to remove the {build} flag on dune dependencies.

@mseri
Copy link
Member

mseri commented Jun 13, 2019

Thanks for the context. I had only partially followed the original discussion, and in fact raised some concerns also for the removal of the build flags.

But as much as I agree that the removal of the {build} flag is disruptive, and that we should probably make opam deal with rebuilding on downgrades, I would favour that disruptive solution instead of disabling important packages.

@avsm
Copy link
Member

avsm commented Jun 13, 2019

I’m not in favour of this PR. My view is that we should get opam repository correct and coinstallable and not worry about a few rebuilds at this stage.

Once we have a version of dune that is forwards compatible we can hopefully apply a safer optimisation in opam. But for now, correctness should come first to avoid breakage on upgrade.

@ghost
Copy link

ghost commented Jun 13, 2019

@avsm dune will not become forward compatible. That's what we discussed yesterday: making dune forward compatible would solve the downgrade problem but would create other more complicated ones.

In the end, it's better to focus our efforts on the shared artifact cache, it will make everything a lot faster and we will make the {build} flag irrelevant for projects using dune.

@avsm
Copy link
Member

avsm commented Jun 13, 2019

That’s completely fine by me. Indeed, binary caching will eliminate a lot of rebuild time, so that’s a good way forward. I’m really keen to see any build unsoundness disappear from opam before we focus on any speed ups at the opam metadata level. The solver is complex enough :-)

@avsm
Copy link
Member

avsm commented Jul 12, 2019

@kit-ty-kate is preparing another PR that will remove {build}, so closing this one

@avsm avsm closed this Jul 12, 2019
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.

4 participants