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.

Sign up for GitHub

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

Jump to bottom

Use Open Watcom to build VirtualBox BIOS #85772

Merged
merged 2 commits into from Apr 28, 2020
Merged

Use Open Watcom to build VirtualBox BIOS #85772

merged 2 commits into from Apr 28, 2020

Conversation

blitz
Copy link
Contributor

@blitz blitz commented Apr 22, 2020 •
edited

Motivation for this change

In order to patch VirtualBox BIOS sources, VirtualBox has to be able to use the (ancient) OpenWatcom C/C++ toolchain to build its BIOS.

This PR adds the latest working Open Watcom release (consisting only of static binaries) and changes VirtualBox to use it to build its BIOS.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

/cc @messemar @tfc @B4dM4n @svanderburg @B4dM4n

@blitz blitz marked this pull request as draft April 22, 2020 13:09
@blitz
Copy link
Contributor Author

blitz commented Apr 22, 2020

The integration test currently fails, but I think it also happens for the vanilla VirtualBox. I'll look into this and maybe convert it to the Python test driver on the way.

@tfc tfc requested a review from B4dM4n April 22, 2020 13:15
@ofborg ofborg bot requested a review from svanderburg April 22, 2020 13:17
@blitz
Copy link
Contributor Author

blitz commented Apr 22, 2020

Mmh. VirtualBox still fails to build with Open Watcom.

@flokli
Copy link
Contributor

flokli commented Apr 24, 2020

The integration test currently fails, but I think it also happens for the vanilla VirtualBox. I'll look into this and maybe convert it to the Python test driver on the way.

I just merged in #85556 - you might want to rebase this PR on top of these changes.

@blitz
open-watcom-bin: init at 1.9
c26e7f9 
This is a derivation for the binary distribution of the Open Watcom
C/C++ compiler.
@blitz blitz marked this pull request as ready for review April 25, 2020 01:41
@blitz blitz requested review from flokli and tfc April 25, 2020 01:43
@blitz
Copy link
Contributor Author

blitz commented Apr 25, 2020

@tfc @flokli I've incorporated your suggestions. I had no luck with the test yet, but I've manually run a Windows 10 VM with no issues.

@blitz
Copy link
Contributor Author

blitz commented Apr 25, 2020

@messemar Maybe you can have a look as well. :)

@blitz
Copy link
Contributor Author

blitz commented Apr 27, 2020

nixpkgs-review is happy:

% nix-shell -p nixpkgs-review --run "nixpkgs-review pr 85772"
[...]
2 package marked as broken and skipped:
linuxPackages_hardkernel_4_14.virtualbox linuxPackages_hardkernel_latest.virtualbox

21 package built:
linuxPackages-libre.virtualbox linuxPackages.virtualbox linuxPackages_4_14.virtualbox linuxPackages_4_19.virtualbox linuxPackages_4_4.virtualbox linuxPackages_4_9.virtualbox linuxPackages_5_5.virtualbox linuxPackages_5_6.virtualbox linuxPackages_hardened.virtualbox linuxPackages_latest-libre.virtualbox linuxPackages_latest_hardened.virtualbox linuxPackages_latest_xen_dom0.virtualbox linuxPackages_latest_xen_dom0_hardened.virtualbox linuxPackages_testing_bcachefs.virtualbox linuxPackages_xen_dom0.virtualbox linuxPackages_xen_dom0_hardened.virtualbox open-watcom-bin virtualbox virtualboxHardened virtualboxHeadless virtualboxWithExtpack

@flokli
Copy link
Contributor

flokli commented Apr 27, 2020

LGTM, but watcom codepath currently isn't used anywhere in nixpkgs, so we won't notice sudden breakages. WDYT about building it with this by default, or at least adding a lowPrio variant of it into all-packages.nix?

@blitz
Copy link
Contributor Author

blitz commented Apr 28, 2020

LGTM, but watcom codepath currently isn't used anywhere in nixpkgs, so we won't notice sudden breakages. WDYT about building it with this by default, or at least adding a lowPrio variant of it into all-packages.nix?

It's currently pulled in as a dependency of VirtualBox. OfBorg has built VBox with Open Watcom:

See: https://logs.nix.ci/?key=nixos/nixpkgs.85772&attempt_id=dc503e7c-8d77-4539-aaba-7a377929a231

Checking for Open Watcom: found version 1.9, OK.

I think the confusion stems from this change:

  • , enableAltBiosSources ? false # Avoids pulling in Open Watcom as a dependency

I chose the comment poorly, because it meant that setting this option to false avoids pulling in the dependency. Will change.

@flokli
Copy link
Contributor

flokli commented Apr 28, 2020 •
edited

Yeah, enableAltBiosSources can be understood the wrong way as well.

What about just making the --with-ow-dir= option conditional on ( open-watcom-bin != null), and add a comment

> # if not passed, virtualbox will fall back to use the shipped alternative sources (assembly)

(as per https://www.virtualbox.org/ticket/12011#comment:3)

@blitz
virtualbox: build BIOS from C instead of alternative sources
7c995a4 
VirtualBox ships with "alternative BIOS sources" for its virtual BIOS.
These are generated by first compiling the BIOS C sources with the
Open Watcom toolchain, disassembling the output and checking in the
disassembly into the VirtualBox repo.

The result means that the BIOS C code cannot be patched, because it's
not compiled from the C sources, if Open Watcom is not there.

As Open Watcom is now available in nixpkgs, we can just ignore the
alternative BIOS sources and compile it from C directly.
@blitz
Copy link
Contributor Author

blitz commented Apr 28, 2020

@flokli Good point. Done.

@flokli flokli merged commit 05328a4 into NixOS:master Apr 28, 2020
@blitz blitz deleted the open-watcom branch April 29, 2020 08:20
@flokli flokli mentioned this pull request Jun 21, 2020
Merged
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tfc tfc tfc approved these changes

@flokli flokli flokli approved these changes

@B4dM4n B4dM4n Awaiting requested review from B4dM4n

@svanderburg svanderburg Awaiting requested review from svanderburg

Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 0 10.rebuild-linux: 11-100
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@blitz @flokli @tfc