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

bochs: move to by-name hierarchy #254284

Merged
merged 1 commit into from
Sep 10, 2023
Merged

bochs: move to by-name hierarchy #254284

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions .../applications/emulators/bochs/default.nix → pkgs/by-name/bo/bochs/package.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,20 @@
, fetchurl
, SDL2
, curl
, darwin
, docbook_xml_dtd_45
, docbook_xsl
, gtk3
, libGL
, libGLU
, libX11
, libXpm
, libobjc
, libtool
, ncurses
, pkg-config
, readline
, wget
, wxGTK
, wxGTK32
Copy link
Member

Choose a reason for hiding this comment

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

Oh this is actually a bit concerning. This is a breaking change because .override { wxGTX = ...; } doesn't work anymore. I don't think we should make these changes until we have a better story for that, even if this allows removing definitions from all-packages.nix.

Note that even if you need custom arguments, you can still use the pkgs/by-name directory.

Copy link
Member Author

@AndersonTorres AndersonTorres Sep 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Bochs didn't update their dependencies yet (they are more concerned about optimizing the emulation core), so overriding wxGTK to other version than that specific one will not work.
I considered the "all-packages-mode" but it didn't feel useful.

Copy link
Member

Choose a reason for hiding this comment

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

Overrides can also be used to just make minor changes to packages, e.g.

.override (old: {
  wxGTK = old.wxGTK.overrideAttrs ...;
})

Similarly the darwin change could also cause breakage, consider:

.override {
  libobjc = ...;
}

These overrides would now silently not do anything anymore.

Of course, Nixpkgs has always not really enforced not having such breakages, but the migration to pkgs/by-name should not cause more of these to happen. This is a problem to be fixed in the future.

I opened #254632 to have this written down.

Copy link
Member Author

Choose a reason for hiding this comment

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

Similarly the darwin change could also cause breakage

That Darwin-specific things were always problematic to me, since the splicing incident (that, among other things, break inherit (X) a b c; design pattern).

Copy link
Member

Choose a reason for hiding this comment

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

Oh yeah that also messes with it, there's definitely many things to be improved. I feel like the custom argument one might be the next one to dig into.

, enableSDL2 ? true
, enableTerm ? true
, enableWx ? !stdenv.isDarwin
Expand Down Expand Up @@ -49,14 +49,14 @@ stdenv.mkDerivation (finalAttrs: {
ncurses
] ++ lib.optionals enableWx [
gtk3
wxGTK
wxGTK32
] ++ lib.optionals enableX11 [
libGL
libGLU
libX11
libXpm
] ++ lib.optionals stdenv.isDarwin [
libobjc
darwin.libobjc
];

configureFlags = [
Expand Down Expand Up @@ -134,17 +134,17 @@ stdenv.mkDerivation (finalAttrs: {

enableParallelBuilding = true;

meta = with lib; {
meta = {
homepage = "https://bochs.sourceforge.io/";
description = "An open-source IA-32 (x86) PC emulator";
longDescription = ''
Bochs is an open-source (LGPL), highly portable IA-32 PC emulator, written
in C++, that runs on most popular platforms. It includes emulation of the
Intel x86 CPU, common I/O devices, and a custom BIOS.
'';
license = licenses.lgpl2Plus;
maintainers = with maintainers; [ AndersonTorres ];
platforms = platforms.unix;
license = lib.licenses.lgpl2Plus;
maintainers = with lib.maintainers; [ AndersonTorres ];
platforms = lib.platforms.unix;
};
})
# TODO: a better way to organize the options
Expand Down
5 changes: 0 additions & 5 deletions pkgs/top-level/all-packages.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2545,11 +2545,6 @@ with pkgs;

basiliskii = callPackage ../applications/emulators/basiliskii { };

bochs = callPackage ../applications/emulators/bochs {
inherit (darwin) libobjc;
wxGTK = wxGTK32;
};

box64 = callPackage ../applications/emulators/box64 {
hello-x86_64 = if stdenv.hostPlatform.isx86_64 then
hello
Expand Down