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

python3Packages.wxpython_4_1: init at 4.1.0 #95462

Closed
wants to merge 1 commit into from

Conversation

tfmoraes
Copy link
Contributor

Motivation for this change

Add WXPython-4.1.0 to NixOS. The package is compiling its own wxgtk because wxpython is not compiling with it.

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"
$ nix build --no-link --keep-going --option build-use-sandbox relaxed -f /home/thiago/.cache/nixpkgs-review/rev-7133bbdbd31696c8a646ea44e85e78f38be3ca2d/build.nix
[2 built, 0.0 MiB DL]
2 packages built:
python37Packages.wxPython_4_1 python38Packages.wxPython_4_1

$ nix-shell /home/thiago/.cache/nixpkgs-review/rev-7133bbdbd31696c8a646ea44e85e78f38be3ca2d/shell.nix

  • 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.

Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

looks like we already have a wxPYTHON_4_0, it would probably be best to only keep one of each major version (unless the package causes breakages between minor versions as well)

pkgs/development/python-modules/wxPython/4.1.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/wxPython/4.1.nix Outdated Show resolved Hide resolved
@tfmoraes
Copy link
Contributor Author

looks like we already have a wxPYTHON_4_0, it would probably be best to only keep one of each major version (unless the package causes breakages between minor versions as well)

There are a lot of change between wxpython4.0.x and wxpython4.1.x. For instance wxWidgets/Phoenix#1612

@tfmoraes
Copy link
Contributor Author

❯ nix-shell -p nixpkgs-review --run "nixpkgs-review rev cc8fd8e62606b751555615985b9af5dc58a79dec"                                                                    
$ git -c fetch.prune=false fetch --force https://github.com/NixOS/nixpkgs master:refs/nixpkgs-review/0
$ git worktree add /home/thiago/.cache/nixpkgs-review/rev-cc8fd8e62606b751555615985b9af5dc58a79dec/nixpkgs 99d379c45c793c078af4bb5d6c85459f72b1f30b
Preparing worktree (detached HEAD 99d379c45c7)
HEAD is now at 99d379c45c7 Merge pull request #79874 from oxzi/tinycbor-v0.5.3
$ nix-env -f /home/thiago/.cache/nixpkgs-review/rev-cc8fd8e62606b751555615985b9af5dc58a79dec/nixpkgs -qaP --xml --out-path --show-trace
$ git merge --no-commit cc8fd8e62606b751555615985b9af5dc58a79dec
Auto-merging pkgs/top-level/python-packages.nix
Automatic merge went well; stopped before committing as requested
$ nix-env -f /home/thiago/.cache/nixpkgs-review/rev-cc8fd8e62606b751555615985b9af5dc58a79dec/nixpkgs -qaP --xml --out-path --show-trace --meta
2 packages added:
python37Packages.wxPython_4_1 (init at 4.1.0) python38Packages.wxPython_4_1 (init at 4.1.0)

$ nix build --no-link --keep-going --option build-use-sandbox relaxed -f /home/thiago/.cache/nixpkgs-review/rev-cc8fd8e62606b751555615985b9af5dc58a79dec/build.nix
[3 built, 0.0 MiB DL]
2 packages built:
python37Packages.wxPython_4_1 python38Packages.wxPython_4_1

@evils
Copy link
Member

evils commented Sep 24, 2020

does this package expose the wxWidgets version it built?
in kicad for example, it'd be nice to be able to use the version of wxWidgets built by this, because it seems to require those packages' versions to match.
(https://gitlab.com/kicad/code/kicad/-/issues/4431)

@tfmoraes
Copy link
Contributor Author

@evils it doesn't. I have this pull request to package wx3.1.4 #95460 . In this commit wxPython starts to use wx3.1.4 wxWidgets/Phoenix@57702fa#diff-abf77184f55403d75b9d51d79162a7ca

@evils
Copy link
Member

evils commented Sep 24, 2020

In this commit wxPython starts to use wx3.1.4 wxWidgets/Phoenix@57702fa#diff-abf77184f55403d75b9d51d79162a7ca

i'm not sure why you're mentioning that
as this is after wxPython 4.1.0
and they've since advanced the dependency to wxWidgets master again

@tfmoraes
Copy link
Contributor Author

@evils I was thinking in package that git-committed version of wxpython, but it's a bad idea. But I managed to compile wxPython-4.1.0 with wxWidgets-3.1.4 in my branch here https://github.com/tfmoraes/nixpkgs/tree/wxgtk314_wxpython411 . Maybe if fix your problem.

@evils
Copy link
Member

evils commented Sep 25, 2020

@tfmoraes your wxPython 4.1.0 with wxWidgets 3.1.4 branch solves kicad issue 4431 :D

@tfmoraes
Copy link
Contributor Author

@evils @jonringer what is better?

  1. Update wxPython and wxWidgets PR and keep 2 PR?
  2. Create a new PR with wxPython and wxWidgets together

@jonringer
Copy link
Contributor

I would just combine the two PRs, you can use either, just make a comment saying what you decide

@evils
Copy link
Member

evils commented Sep 27, 2020

Since this seems to copy the previous wxPython package, have you taken into account #94108?
A lot of the previous package's dependencies were unneeded

("appsvc", None)
]}'

# https://github.com/wxWidgets/Phoenix/pull/1584
Copy link
Member

Choose a reason for hiding this comment

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

it's probably better to refer to wxWidgets/Phoenix#1699 as that indicates this change has been upstreamed and can probably be removed at a later point

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, in the other PR #98951

@tfmoraes
Copy link
Contributor Author

I would just combine the two PRs, you can use either, just make a comment saying what you decide

Hi @jonringer, I created a new PR with wxWidgets + wxPython #98951

@tfmoraes
Copy link
Contributor Author

Since this seems to copy the previous wxPython package, have you taken into account #94108?
A lot of the previous package's dependencies were unneeded

Yes, @evils in the other PR with both changes #98951

@tfmoraes tfmoraes closed this Oct 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants