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

wxwidgets-kicad-macos init at 3.0.4 #98538

Closed
wants to merge 1 commit into from

Conversation

hannesweisbach
Copy link
Contributor

Motivation for this change

KiCad uses a patched wxWidgets on macOS. KiCad maintains a branch of
wxWidgets with all patches already applied. This package provides wxWidgets
solely for KiCad on macOS.

See #98450
Possibly interesting here: #90654

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.

KiCad uses a patched wxwidgets on macOS. KiCad maintains a repository of
wxwidgets with all patches already applied. This package provides wxwidgets
solely for KiCad on macOS.
@veprbl
Copy link
Member

veprbl commented Sep 25, 2020

Seems like #97101 is aimed at the same goal of providing kicad on macOS and does not require a new package.

cc @fricklerhandwerk

@hannesweisbach
Copy link
Contributor Author

hannesweisbach commented Sep 25, 2020

#97101 is about building wxPython_4_0 on Darwin/macOS. While wxPython_4_0 is an (optional) depedency for KiCad and the wxWidgets versions of wxPython and KiCad have to match (https://gitlab.com/kicad/code/kicad/-/issues/4431, see also #98634) it does not negate the requirement of a specifcally-patched version of wxWidgets on macOS for KiCad, imho …

wxPython_4_0 compiles fine for me on macOS with the wxWidgets-version required by KiCad. I've been doing that for weeks.

Edit: I hope the special-patch-situation goes away in the future and KiCad can use a stock wxWidgets on macOS too, but that is a question for the KiCad-devs.

@evils evils mentioned this pull request Dec 24, 2020
10 tasks
, libGLU, libGL
, compat24 ? false, compat26 ? true
, setfile, AGL, Carbon, Cocoa, Kernel, QTKit, WebKit, AVFoundation, AVKit
, ...
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
, ...

, ...
}:

with stdenv.lib;
Copy link
Member

Choose a reason for hiding this comment

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

I am not a fan of this. It would be better to apply this on a smaller scale.

setfile Kernel QTKit WebKit AVFoundation AVKit AGL
];

nativeBuildInputs = [ pkgconfig ];
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
nativeBuildInputs = [ pkgconfig ];
nativeBuildInputs = [ pkg-config ];

# purpose of this package. Hopefully, this will eventually go away and KiCad
# will use a stock wxwidgets on macOS, obviating the need for this packages.

{ stdenv, fetchgit, fetchurl, pkgconfig
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{ stdenv, fetchgit, fetchurl, pkgconfig
{ stdenv, fetchgit, fetchurl, pkg-config

[ "--disable-precomp-headers" "--enable-mediactrl"
(if compat24 then "--enable-compat24" else "--disable-compat24")
(if compat26 then "--enable-compat26" else "--disable-compat26")
"--with-cocoa" "--with-macosx-version-min=10.10"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"--with-cocoa" "--with-macosx-version-min=10.10"
"--with-cocoa" "--with-macosx-version-min=10.12"

We should probably set this to apple_sdk version in darwin dynamically.

Comment on lines +38 to +43
[ "--disable-precomp-headers" "--enable-mediactrl"
(if compat24 then "--enable-compat24" else "--disable-compat24")
(if compat26 then "--enable-compat26" else "--disable-compat26")
"--with-cocoa" "--with-macosx-version-min=10.10"
"--enable-webview" "--enable-webview-webkit"
"--enable-unicode" "--with-opengl"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
[ "--disable-precomp-headers" "--enable-mediactrl"
(if compat24 then "--enable-compat24" else "--disable-compat24")
(if compat26 then "--enable-compat26" else "--disable-compat26")
"--with-cocoa" "--with-macosx-version-min=10.10"
"--enable-webview" "--enable-webview-webkit"
"--enable-unicode" "--with-opengl"
[
"--disable-precomp-headers"
(if compat24 then "--enable-compat24" else "--disable-compat24")
(if compat26 then "--enable-compat26" else "--disable-compat26")
"--enable-mediactrl"
"--enable-unicode"
"--enable-webview"
"--enable-webview-webkit"
"--with-cocoa"
"--with-macosx-version-min=10.10"
"--with-opengl"

Comment on lines +49 to +55
substituteInPlace configure --replace 'SEARCH_INCLUDE=' 'DUMMY_SEARCH_INCLUDE='
substituteInPlace configure --replace 'SEARCH_LIB=' 'DUMMY_SEARCH_LIB='
substituteInPlace configure --replace /usr /no-such-path
substituteInPlace configure --replace \
'ac_cv_prog_SETFILE="/Developer/Tools/SetFile"' \
'ac_cv_prog_SETFILE="${setfile}/bin/SetFile"'
substituteInPlace configure --replace "-framework System" -lSystem
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
substituteInPlace configure --replace 'SEARCH_INCLUDE=' 'DUMMY_SEARCH_INCLUDE='
substituteInPlace configure --replace 'SEARCH_LIB=' 'DUMMY_SEARCH_LIB='
substituteInPlace configure --replace /usr /no-such-path
substituteInPlace configure --replace \
'ac_cv_prog_SETFILE="/Developer/Tools/SetFile"' \
'ac_cv_prog_SETFILE="${setfile}/bin/SetFile"'
substituteInPlace configure --replace "-framework System" -lSystem
substituteInPlace configure \
--replace 'SEARCH_INCLUDE=' 'DUMMY_SEARCH_INCLUDE=' \
--replace 'SEARCH_LIB=' 'DUMMY_SEARCH_LIB=' \
--replace /usr /no-such-path \
--replace 'ac_cv_prog_SETFILE="/Developer/Tools/SetFile"' 'ac_cv_prog_SETFILE="${setfile}/bin/SetFile"' \
--replace "-framework System" -lSystem

'';

postInstall = "
(cd $out/include && ln -s wx-*/* .)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
(cd $out/include && ln -s wx-*/* .)
pushd $out/include
ln -s wx-*/* .
popd

Comment on lines +70 to +75
meta = rec {
description = "Patched version of wxWidgets for KiCad on macOS";
homepage = "https://gitlab.com/kicad/code/wxWidgets";
license = stdenv.lib.licenses.unfree;
platforms = stdenv.lib.platforms.darwin;
};
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
meta = rec {
description = "Patched version of wxWidgets for KiCad on macOS";
homepage = "https://gitlab.com/kicad/code/wxWidgets";
license = stdenv.lib.licenses.unfree;
platforms = stdenv.lib.platforms.darwin;
};
meta = with stdenv.lib; {
description = "Patched version of wxWidgets for KiCad on macOS";
homepage = "https://gitlab.com/kicad/code/wxWidgets";
license = licenses.unfree;
platforms = platforms.darwin;
};

@veprbl
Copy link
Member

veprbl commented Jan 6, 2021

I don't think we need to reformat this as this is largely a copy-pasta of default.nix. I suppose, it would be better to define this via wxGTK30-gtk3.overrideAttrs.

@stale
Copy link

stale bot commented Jul 8, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 8, 2021
@SuperSandro2000
Copy link
Member

Closing due to inactivity from author.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: darwin Running or building packages on Darwin 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants