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

hyperkit 0.20210107 #72028

Closed
wants to merge 1 commit into from
Closed

hyperkit 0.20210107 #72028

wants to merge 1 commit into from

Conversation

Bo98
Copy link
Member

@Bo98 Bo98 commented Feb 26, 2021

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

@BrewTestBot BrewTestBot added the ocaml OCaml use is a significant feature of the PR or issue label Feb 26, 2021
@Bo98
Copy link
Member Author

Bo98 commented Feb 26, 2021

Testing a VM inside of a VM (the CI) is probably going to be a problem here.

@mitchblank
Copy link
Contributor

Was there some progress on Big Sur that isn't yet reflected on moby/hyperkit#297 ?

@Bo98
Copy link
Member Author

Bo98 commented Feb 28, 2021

Yes, moby/hyperkit#307

@carlocab carlocab mentioned this pull request Mar 16, 2021
@BrewTestBot
Copy link
Member

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@BrewTestBot BrewTestBot added the stale No recent activity label Mar 22, 2021
@carlocab
Copy link
Member

If we can't test this in a sandbox, we may as well just get rid of the test block (or put in some trivial test that will work in a sandbox).

The current bottles are failing tests anyway...

@BrewTestBot BrewTestBot removed the stale No recent activity label Mar 22, 2021
@mitchblank
Copy link
Contributor

If we can't test this in a sandbox, we may as well just get rid of the test block (or put in some trivial test that will work in a sandbox).

IMO the best option would be to leave the test but skip it on CI -- that way it at least can be run manually as part of formula maintenance. However I haven't had luck in the past with getting changes like that past review: #66485 (comment)

@github-actions
Copy link
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale No recent activity label Apr 13, 2021
@Bo98
Copy link
Member Author

Bo98 commented Apr 13, 2021

There's been some changes so the VM environment so let's see if anything's changed here.

Comment on lines 45 to 50
system "opam", "exec", "--",
"opam", "install", "uri.4.1.0", "qcow.0.11.0", "conduit.2.1.0", "lwt.5.3.0",
"qcow-tool.0.11.0", "mirage-block-unix.2.12.0", "conf-libev.4-11", "logs.0.7.0", "fmt.0.8.9",
"mirage-unix.4.0.0", "prometheus-app.1.0"
Copy link
Member

Choose a reason for hiding this comment

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

Would be good to add a comment here about how to update these when needed.

@Bo98 Bo98 removed the stale No recent activity label Apr 13, 2021
@Bo98
Copy link
Member Author

Bo98 commented Apr 14, 2021

# Error: /usr/local/lib/ocaml/findlib/topfind.cmi
#        is not a compiled interface for this version of OCaml.

Did we not revsion bump ocaml-findlib when updating OCaml?

@carlocab
Copy link
Member

Unfortunately not: #71942

But CI didn't pick anything up with ocaml-findlib. ocaml-findlib was even used to build other formulae in the same PR.

@Bo98
Copy link
Member Author

Bo98 commented Apr 14, 2021

But CI didn't pick anything up with ocaml-findlib. ocaml-findlib was even used to build other formulae in the same PR.

I suppose if it only uses the binary then there's no issues. There'll only be issues if it actually uses it as a library, and opam by default does not use system findlib (which the extra lines in this formula show).

This is certainly the first formula to use the topfind part.

@carlocab
Copy link
Member

Oops. We should probably add ocaml-findlib to the comment at the start of the ocaml formula then.

@Moisan Moisan mentioned this pull request Apr 22, 2021
@github-actions
Copy link
Contributor

github-actions bot commented May 6, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale No recent activity label May 6, 2021
@github-actions github-actions bot closed this May 13, 2021
@cho-m cho-m reopened this May 26, 2021
@cho-m cho-m removed the stale No recent activity label May 26, 2021
@cho-m
Copy link
Member

cho-m commented May 27, 2021

Tried another run with updated ocaml-findlib, but now back to failing on testcase again.

EDIT: Haven't been able to get test to work locally, though I am trying via Rosetta layer.
Maybe someone more familiar with hyperkit can have a go at this.

@gromgit gromgit mentioned this pull request Jun 20, 2021
@github-actions
Copy link
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale No recent activity label Jun 24, 2021
@github-actions github-actions bot closed this Jul 1, 2021
@github-actions github-actions bot added the outdated PR was locked due to age label Aug 3, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ocaml OCaml use is a significant feature of the PR or issue outdated PR was locked due to age stale No recent activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants