Skip to content
This repository has been archived by the owner on Oct 29, 2022. It is now read-only.

shared/opam_install.sh is a heresy #5

Closed
maximedenes opened this issue Mar 30, 2017 · 6 comments
Closed

shared/opam_install.sh is a heresy #5

maximedenes opened this issue Mar 30, 2017 · 6 comments

Comments

@maximedenes
Copy link
Member

Correct me if I'm wrong, but with shared/opam_install.sh, if the network is down for 15 minutes, we'll get results without failures, but with a 15 minutes increment in the timings. That's a terrible idea, and that's probably what happend with fiat some time ago.

@ghost
Copy link

ghost commented Mar 30, 2017

Good point.

@ghost
Copy link

ghost commented Mar 30, 2017

Two things (at least) need to be done:

  • avoid non-terminating loop in case of permanent network errors
  • make sure that if transient network errors occur, the benchmarking time is not disturbed.

This is related to two problems:

  • originally, when some of the opam install commands failed, the whole thing failed
  • there is no direct way to separate the download action from the build and install actions

If they were separable, I could wrap the download action with a loop that would deal with transient networking problems so that we can measure the time of build independently from the time how long does it take to download a given thing. This is related to issue #1 .

ghost pushed a commit that referenced this issue Mar 30, 2017
…ges.

As objected here:

  #5

it is a bad idea to do that.
@ghost
Copy link

ghost commented Mar 30, 2017

This should be fixed now.

My very rudimentary tests have finished. No gross errors were discovered with this commit.

@ghost
Copy link

ghost commented Mar 30, 2017

I still use the heretic opam_install.sh in those places where it is harmless, though.

@maximedenes
Copy link
Member Author

I still use the heretic opam_install.sh in those places where it is harmless, though.

:) You mean to install dependencies? It would be good to limit the number of attempts in any case (like 3).

@ghost
Copy link

ghost commented Mar 30, 2017

Ok. I'll make that change.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant